Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[GLUTEN-3637][VL] Fix whole stage pipeline metric #3661

Merged
merged 1 commit into from
Nov 10, 2023
Merged

[GLUTEN-3637][VL] Fix whole stage pipeline metric #3661

merged 1 commit into from
Nov 10, 2023

Conversation

Yohahaha
Copy link
Contributor

@Yohahaha Yohahaha commented Nov 9, 2023

(Fixes: #3637)

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 9, 2023

#3637

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 9, 2023

Run Gluten Clickhouse CI

zhztheplayer
zhztheplayer approved these changes Nov 9, 2023
View reviewed changes
Copy link
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching!

zhouyuan
zhouyuan approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@zhouyuan zhouyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 works for me

@zhouyuan zhouyuan merged commit 33003d4 into apache:main Nov 10, 2023
16 checks passed
zhouyuan
zhouyuan reviewed Nov 10, 2023
View reviewed changes
gluten-core/src/main/scala/io/glutenproject/utils/Iterators.scala
pipelineTime += accumulatedTime.getAndSet(
0L
pipelineTime += TimeUnit.NANOSECONDS.toMillis(
accumulatedTime.getAndSet(
Copy link
Contributor

@zhouyuan zhouyuan Nov 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is due to accumulatedTime is changed during these days and it's nano-seconds(should use milliseconds)?
CC: @JkSelf

@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_3661_time.csv log/native_master_11_09_2023_745ecb383_time.csv difference percentage
q1 33.92 33.80 -0.116 99.66%
q2 24.37 24.95 0.583 102.39%
q3 36.54 38.87 2.325 106.36%
q4 37.61 36.96 -0.651 98.27%
q5 70.77 69.94 -0.829 98.83%
q6 8.12 7.87 -0.248 96.94%
q7 83.59 85.40 1.812 102.17%
q8 88.48 88.61 0.129 100.15%
q9 121.12 121.14 0.016 100.01%
q10 48.26 54.35 6.086 112.61%
q11 20.19 19.52 -0.672 96.67%
q12 26.38 24.93 -1.455 94.48%
q13 48.27 50.92 2.654 105.50%
q14 19.52 18.17 -1.358 93.04%
q15 33.34 30.79 -2.548 92.36%
q16 16.57 16.06 -0.507 96.94%
q17 101.81 101.51 -0.300 99.71%
q18 147.02 148.23 1.210 100.82%
q19 14.79 14.95 0.163 101.10%
q20 30.16 31.24 1.077 103.57%
q21 226.34 223.30 -3.033 98.66%
q22 14.26 14.24 -0.018 99.88%
total 1251.42 1255.74 4.319 100.35%

@Yohahaha Yohahaha deleted the fix-pipeline-metric branch November 10, 2023 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhouyuan zhouyuan zhouyuan approved these changes

@zhztheplayer zhztheplayer zhztheplayer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WholeStage pipeline metric was broken
4 participants
@Yohahaha @GlutenPerfBot @zhouyuan @zhztheplayer